Skip to content

Conversation

@XhmikosR
Copy link
Member

@XhmikosR XhmikosR commented Apr 7, 2014

The test failures I was getting after switching to === and !== were actually caused by wrong checks in the beginning.

Example:
event.value == "none" when value isn't a string.

For the quotmark I went with double quotes since that's what was already being used more often.

@XhmikosR
Copy link
Member Author

XhmikosR commented Apr 7, 2014

Only thing broken is testrunner.htm. I'll try to fix this soon-ish.

@XhmikosR
Copy link
Member Author

XhmikosR commented Apr 7, 2014

Fixed.

@nschonni
Copy link
Member

nschonni commented Apr 7, 2014

Thanks @XhmikosR ! I'm still debating whether some of these should wait for a JSCS cleanup (need to wait for the next cut for some indenting bugs in 1.3.0)

@nschonni nschonni added the Triage label Apr 7, 2014
@XhmikosR
Copy link
Member Author

XhmikosR commented Apr 8, 2014

I wouldn't wait for JSCS at this point since JSCS is for style only....

@XhmikosR
Copy link
Member Author

XhmikosR commented Apr 8, 2014

Also, keep in mind that it's hard to keep rebasing this branch so we better get this in. It's also blocking me from making other changes like sorting the destination build dir etc.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should be removed entirely since it looks like it was related to some previously removed code in e91d7ef

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, you are right, I'll remove it.

@nschonni
Copy link
Member

nschonni commented Apr 9, 2014

@XhmikosR I get the frustration from rebasing, and I know this isn't the first time you've heard this, but smaller PRs are easier to review and merge.

@XhmikosR
Copy link
Member Author

XhmikosR commented Apr 9, 2014

I can break the patches, but they all will conflict so I'll need to open one PR at a time. That's fine as long as the PRs are merged sort of fast :)

@XhmikosR
Copy link
Member Author

@nschonni: now this PR only has the strict changes.

@XhmikosR
Copy link
Member Author

Now, one thing I was wondering about, should we call toLowercase() in the places toString() is used?

@XhmikosR
Copy link
Member Author

XhmikosR commented May 2, 2014

bump

nschonni added a commit that referenced this pull request May 2, 2014
@nschonni nschonni merged commit 5d69c54 into CSSLint:master May 2, 2014
@nschonni nschonni added this to the 0.11.0 milestone May 2, 2014
@XhmikosR XhmikosR deleted the jshint branch May 3, 2014 06:15
@XhmikosR XhmikosR modified the milestones: 0.11.0, 1.0.0 Jan 9, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants